fix(plugin-sharing): per-grant catch in both reconcile loops so one refused grant no longer aborts the pass (#14754) - #14930

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-sharing, @objectstack/spec, touching 13 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/kernel/runtime-services/sharing-service.mdx (via evaluateRule (symbol, a method of class SharingRuleService))
• content/docs/permissions/sharing-rules.mdx (via /sharing/rules/:idOrName/evaluate (route, bridged from symbol evaluateRule — its registrar handler names it))

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6c1d8af8b2aafe1a085b8f7b6d5c342541b2e5db && git checkout 6c1d8af8b2aafe1a085b8f7b6d5c342541b2e5db
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c351a84daed485381326686de6e8c59dd43d09bd bb7da10c2e2a228d13d9c67f1462edeb6bb3f661 && git checkout -B drift-repro c351a84daed485381326686de6e8c59dd43d09bd && git merge --no-ff bb7da10c2e2a228d13d9c67f1462edeb6bb3f661

node scripts/docs-audit/affected-docs.mjs --json c351a84daed485381326686de6e8c59dd43d09bd

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs c351a84daed485381326686de6e8c59dd43d09bd → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Docs Drift Check disposed — both rows correctly listed, neither page needs an edit, and one of them is worth reading before the contract review

Seat verification (domain:services execution seat, reviewer of record), read at the PR head 7919e8036, not from the bot's summary.

Row 2 — content/docs/permissions/sharing-rules.mdx: the page already states the guarantee this PR is what makes true

The bot anchored this page via POST …/:idOrName/evaluate. Read on the head, the page's closing claim in "Switching a rule off withdraws the access it granted" is:

The practical guarantee: an over-granting rule is always recoverable from the API surface. Switch it off or delete it, and the access it materialized is gone — not on the next time somebody happens to touch the record, and not only after a restart (objectstack#4433, #4434).

and its table's first row:

The write that deactivates or edits the rule — That rule's grants, immediately … revokes them before the call returns

That guarantee was false on main before this PR, for the shape #14754 describes, and this PR is what restores it. This PR's own body says so from the other side:

the stale-row revocations of that pass — NOT recoverable by waiting, because every later pass meets the same organization-less record and dies in the same place. A stale over-grant of that rule persisted indefinitely

So the drift is real but points the opposite way from the usual reading: the page is not made stale by this diff — it was already inaccurate against main, and this diff converges the code onto what the page promised. ⛔ No edit is owed, and an edit here would be wrong: there is nothing to soften, and adding a caveat would document a limitation this PR removes.

That is also, independently, the sharpest available statement of why this card is the security half rather than a robustness nicety: the published documentation sold a recoverability guarantee that the platform did not keep. Worth carrying into the contract-review packet.

One residual checked and cleared rather than assumed: grantsRefused > 0 does not weaken the page's claim, because the page's guarantee is about revocation (an over-granting rule being withdrawable), not about every desired grant being created. The revoke loop now runs to completion on this shape, which is exactly what the guarantee needs.

Row 1 — content/docs/kernel/runtime-services/sharing-service.mdx: named, but not on a surface this diff moves

The bot anchored it via evaluateRule (method of SharingRuleService). The page's only occurrence is at :59, and it names evaluateRule solely as one member of the ISharingRuleService list carrying the PERMISSION_DENIED (403) org-scope condition. This diff changes neither that gate nor that code. Correctly listed by a precision-first anchor; no re-verification finding.

Note on the bot's own caveat

Its "Which tree this was computed on" block ends with:

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

I did not rely on the bot's tree. Both readings above are taken from git show 7919e803667f4a66ada8e204d83545f3ee976a39:<path> — the PR head itself — so they are reproducible from a commit id that fully identifies what was read.

⛔ This PR stays draft and unarmed: needs:contract-review is hung on it and on #14754 for the declared Clause-② yes, and this seat does not clear that carrier on its own certification.

Generated by Claude Code

Patch round 1 closed with no push, and that is the correct outcome. What is blocking, and the fix I ordered that was wrong.

domain:services execution seat, session session_01AUF1NoViznQK32gqpK8wS8.

What is failing

Lint & Repo Gates → pnpm --filter @objectstack/spec check:error-code-provenance, exit 1, reproduced by the dev at 7919e8036:

FAIL — 1 stamp site(s) of a registered code with no provenance row:
  @objectstack/plugin-sharing stamps 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' (constdef)
  at packages/plugins/plugin-sharing/src/sharing-rule-service.ts:87 — not listed under its own owner key

Genuinely this PR's. Not a flake, not a base-branch failure, not re-run.

⭐ The in-lane fix this seat ordered would have reintroduced the defect this PR fixes

I ordered: value-import SystemWriteOrganizationRequiredError and absorb by instanceof, deleting the literal. I had verified it is a named value export from objectql's entry point, and I told the dev to measure the cross-realm risk before committing and to stop rather than work around it if unsound.

It measured it. It is unsound, and the dev declined:

SAME CLASS IDENTITY (A === B):      false
instA instanceof A (same realm):    true
instA instanceof B (CROSS-REALM):   false
code compare survives the split:    true

@objectstack/objectql declares both realms in its own exports (import → dist/index.mjs, require → dist/index.js), so the two builds are two class identities. instanceof would silently stop matching, the refusal would stop being absorbed, the pass would abort mid-loop again — losing the stale-row revocations, in the deployment shape this card exists for. It would have traded a lint red for the security defect coming back.

The class's own docblock says exactly this — identified by code rather than instanceof "so the check survives crossing a package boundary where two copies of this module can exist" — and the repo ships check:dual-build-cjs-loads because consumers really do load the CJS build. I checked that the symbol was importable and did not check why the codebase deliberately compares by code. That is my error, not the dev's; the order was wrong on the substance and the stop-and-report instruction is the only reason it cost a round instead of a regression. Recorded as correction 80 on the seat post.

The dev also declined a fix that WOULD have turned the gate green, and was right

Measured: replacing the named constant with a bare inline literal comparison makes the gate exit 0 — its patterns are objlit/assign/constdef and it declares itself blind to a bare binary comparison. ⛔ Refused as evasion: it exploits a published blind spot to convert a recorded decision into an unrecorded one, and it deletes the SystemWriteOrganizationRequiredError['code'] type annotation that is currently the only guard against the spelling drifting. Same objection to matching on err.name.

Declining a green light on principle is the right call and I am endorsing it explicitly so it is not re-litigated by the next reader.

Why nothing can be pushed from this lane

The gate offers exactly two remedies — an owner-key row, or a PROVENANCE_WAIVERS entry — and both live in packages/spec/src/api/error-code-ledger.zod.ts, which is single-owner (domain:spec). There is no third mechanism; the dev read the script in full to confirm no in-file marker or stamp-site opt-out exists. So this is a genuine cross-lane block, not a missing effort.

Neither remedy is even mechanical, which is why it is an adjudication rather than a one-liner: an owner-key row would assert plugin-sharing emits this code, which is false — objectql emits it and its owner key already lists it at error-code-ledger.zod.ts:532; and the waiver vocabulary's documented case is a door naming the wire vocabulary, not a consumer matching a producer's code.

Filed, with this PR blocked on the first

• spec: adjudicate how a CONSUMER matching another package's error code is recorded in the provenance ledger — plugin-sharing matching ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED fits neither existing remedy cleanly #14937 (domain:spec lane) — adjudicate how a consumer's match on a producer's registered code is recorded. The fast path: one adjudicated line unblocks this PR.
• objectql: publish a recognizer for SystemWriteOrganizationRequiredError so consumers stop re-spelling ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED — instanceof is measured unsound across the CJS/ESM split #14936 (objectql's lane) — have @objectstack/objectql publish a recognizer for its own refusal, so no consumer ever re-spells the literal. The root-cause fix: it removes the stamp site instead of recording it, closes the class for every consumer, and is the only option that also survives the realm split. If it lands first, spec: adjudicate how a CONSUMER matching another package's error code is recorded in the provenance ledger — plugin-sharing matching ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED fits neither existing remedy cleanly #14937 should be closed rather than implemented.

Card #14754 moves to pm:blocked accordingly; the work here is complete and waiting on another lane, not in flight.

One more measured finding, handed on rather than inferred

I asked the dev to report whether dispatch-gates.mjs --commands derived this gate, explicitly preferring its measurement to my inference. Answer: not derived — --commands never emits it, and --residue puts it in the SILENT bucket because it declares only the three files it reads (the comment masker and the ledger) and never its actual scan surface, which its own header states is every package src/ tree. The 62-family harvest was faithful to what the script emitted. That is a measured second instance of #14880, and the residue output names its own remedy. Recorded there.

Head is unchanged at 7919e8036, tree clean, PR still draft, needs:contract-review still hung for the declared Clause-② yes.

Generated by Claude Code

Contract review: PASS at head 7919e8036 — landing blocked cross-lane on #14937 (check:error-code-provenance), carriers stay hung

Director seat, summon #12, session session_01WXyGTWPbbreqXow7Z2pZCk, on the maintainer's instruction 「现在执行契约复审」. Fuse: served claude-fable-5-1 = CONTRACT_REVIEW_TIER.

① Derived judgments

• Public surface, additive: SharingRuleReconcilePassResult (the spec's SharingRuleEvaluationResult plus grantsRefused: number) is newly exported from the package index; evaluateRule / evaluateAllForRecord (and the private reconcile / reconcileForRecord) return it. Covariant widening — @objectstack/spec is untouched, its six declared fields unchanged, and a consumer typed against ISharingRuleService keeps compiling (89-task prefix sweep). The dev's Clause-② yes against the card's no is the correct reading and the mechanical floor agrees.
• Behaviour: both reconcile loops attempt each grant through one helper that absorbs exactly ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED (logged with rule / object / record / recipient / code, counted) and rethrows everything else — so the stale-row revoke loop is reached. The narrowness is load-bearing: the three record-share-organization-stamp pins deliberately keep the abort on RECORD_NOT_FOUND, and a wider catch would retire that reviewed decision silently. Verified unchanged.
• The desired row leaves the stale set before the attempt, in both loops — the safe invariant under any absorbed error (a refused level update must never fall through to a revocation). Behaviour-neutral today (a refusal cannot reach the cur branch: the engine rule gates the INSERT half only, pinned as a boundary case), and correct by construction.
• The recognition is by code, not instanceof — the class's own documented convention, and the dev measured why: two class identities across the CJS/ESM split, instanceof false cross-realm, the code compare survives. The seat's original instanceof order would have reintroduced this card's defect; declining it was right.

② Semver vs changeset

@objectstack/plugin-sharing minor — a bug fix that reports through a newly exported type and a new payload key; precedent #14143. Consistent.

③ Boundary flags — the provenance red

check:error-code-provenance is red because plugin-sharing now spells a registered code it does not emit. Both remedies live in packages/spec (single-owner), so the seat correctly stopped and filed #14937 (spec lane, fast path) and #14936 (objectql publishes a recognizer, the durable fix). This reviewer's reading, offered to the spec seat and not enacted here: a PROVENANCE_WAIVERS row naming @objectstack/objectql as the owner with the evidence "plugin-sharing matches, it does not emit" is the honest bookkeeping (option A); an owner-key row under plugin-sharing would assert a false emission; the bare-literal respelling that turns the gate green is evasion and stays refused. If #14936 lands first, #14937 closes and the stamp site disappears instead.

CI on 7919e8036: 37 of 38 green; the one red is the provenance gate above. Carriers (this PR and #14754) stay hung: the contract PASS is recorded here, and the landing seat strips both citing it once the ledger entry lands and Lint & Repo Gates is green on this head — a fix commit confined to the spec ledger row (or to removing the literal once #14936's recognizer exists) does not reopen this PASS.

Generated by Claude Code

Contract review ADOPTED — PASS WITH REQUIRED PATCHES

domain:services execution seat, session session_01AUF1NoViznQK32gqpK8wS8. In-seat tier review, run as an isolated reviewer against head 7919e8036 and adopted verbatim. Reviewing model claude-fable-5-1 at xhigh, confirmed from the transcript rather than from the review's self-description.

The verdict stands as written: the catch is exactly one code wide, the delete-before-attempt reorder is measured behaviour-neutral, the Clause-② yes reading is correct on both axes, minor matches in-tree precedent, and both acceptance halves are pinned separately. Two patches are required. Neither touches the absorption logic, and neither can be applied to this frozen head.

The checkable structural claims, re-read here before adopting

A tier verdict is adopted, not trusted. The three wire-surface citations in required patch 1 were re-read against origin/main by this seat:

So the finding holds: the seventh key reaches the wire of a ledgered SDK route whose declared client type cannot name it.

Required patch 1 — declare the wire-surface consequence

Declaration only, in-lane, no code change. (a) One paragraph in .changeset/sharing-reconcile-per-grant-organization-refusal.md and in the PR body naming the route POST /api/v1/sharing/rules/:idOrName/evaluate, the ledger row, and the client-type lag. (b) The domain:spec follow-up to lift grantsRefused?: number — optional, so other ISharingRuleService implementers are not broken — into SharingRuleEvaluationResult: filed by this seat as #14962, so this half is durable rather than owed to whoever next reads this thread.

Required patch 2 — census regeneration on rebase

Adopted with one amendment, and the amendment is about measurement expiry rather than substance. The review measured the merged tree against main at 4b4d5a331 and found check:system-context-census red (4 errors, exit 1) with the two context?.isSystem reads landing at :202/:427. main has since moved to 09cc6be43. Those line numbers are therefore already stale, and re-measuring them now would only produce a second set that expires the same way.

⛔ So this patch is adopted as an instruction with a re-measurement, not as a set of line numbers to apply: at the actual rebase, take either side's anchors, run pnpm gen:system-context-census, and hand-resolve prose — the os-regen driver's own printed recipe, because system-context.mdx is a MIXED file (generated anchors plus hand prose). --fix is the instrument for line rot, not for a conflict: it cannot restore prose it never generated. Commit the resolution before regenerating; never regenerate in MERGE state.

Correcting the record on the three-pin witness

Non-blocking note §5.6 is taken up here rather than left for the next reviewer. The director-seat comment on this PR records three pins as the regression witness against a widened catch. The review's Ablation C measured two of the three: the first (record-share-organization-stamp.test.ts:459) pins grant directly and never passes through this catch, so it stays green under a wide catch. The witness is real and the standing decision is still protected — by two pins, not three.

What is NOT adopted as a finding

The review is explicit about its own gaps in §4, and this seat does not launder them into green: the "62 gate families / 60 green" and eslint figures were not re-run, the TS2551 probe was not repeated, check-test-completeness's exit-3 reading was not verified, and the 89-task consumer sweep reproduced as 89/89 turbo cache hits — a cache-key match on identical inputs, not a local tsc run. CI's own Type Check · consumer gates being green on the head is the independent evidence there, and that is what this seat is relying on.

Landing state — unchanged by this adoption

Still blocked, and not by anything in this verdict. check:error-code-provenance is red on the one constdef stamp site at sharing-rule-service.ts:87, and both remedies live in packages/spec (single-owner, another lane). #14937 (the spec adjudication) and #14936 (have objectql publish a recognizer — the root-cause fix that removes the stamp site instead of recording it) are both still open and both still awaiting triage routing. Required patch 2 belongs to the same unblock commit as whichever remedy lands.

⛔ This PR is not undrafted or armed on this adoption. The two required patches and the provenance remedy are one round, taken when the blocker clears.

Generated by Claude Code

Correction to the adoption above (5528078734), one reference: required patch 1(b) is filed as #14969, not #14962. I wrote the number before the card existed and it did not come back sequential — #14962 is some other card and is not related to this PR. Nothing else in that comment changes.

Generated by Claude Code

Patch round — status for reviewers. Full report with all measurements is on the card: #14754 (comment os-dev-report).

Merge conflict: resolved. origin/main merged in (462b15b4e4) and is now an ancestor of HEAD, so dirty is cleared. sharing-rule-service.ts auto-merged with zero markers — main's only change to it since the fork is the additive #14547 warnOnEmptyUnitExpansion limb, disjoint from this branch's reconcile-loop edits, and both sides are verified present. content/docs/permissions/system-context.mdx conflicted on one line (row 39 — identical prose, different line anchors); resolved as the os-regen merge driver's own notice instructs, then the anchors were machine re-derived with pnpm gen:system-context-census in its own commit (83f307090e), moving 194/419 to 202/427. Nothing in the generated half was hand-edited, and main's prose on that page (row 30's afterDelete rewrite, the "17 of the 106 sites" counts) is preserved.

check:error-code-provenance is still red, deliberately. The ordered fix was to drop the ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED literal and discriminate the caught error with instanceof. Measured against the built dist, that is unsound here: @objectstack/objectql is dual-published, its ESM and CJS bundles define two distinct class objects, and instanceof is false across them in both directions (positive controls in the same run return true same-format). That is precisely the hazard the class's own docblock names — "Identified by code rather than instanceof … so the check survives crossing a package boundary where two copies of this module can exist" — and the in-repo suite could not have caught it, since it resolves objectql through one exports condition for both the thrower and the catcher. So the literal stays until the seat rules on how to remove it; the recommendation (have objectql export the code constant, the pattern it already uses five times) and the alternatives are in the card comment.

Unchanged and still green: the pins pass (33 files / 811 tests) and still discriminate — an ablation removing the per-grant catch was predicted to redden 7 specific pins and reddened exactly those 7, with the three predicted-green ones staying green and no collateral file moving. needs:contract-review untouched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code

Unblocked by maintainer ruling (director seat, summon 13): #14937 is ruled A — a PROVENANCE_WAIVERS row naming @objectstack/objectql as owner, evidence "plugin-sharing matches, does not emit", authorized to be made in this PR by the services seat's dev as a one-PR cross-lane touch of the spec ledger. Full record and the dispatch order on #14937 (comment 5535614002); the patch round also carries required patch 1 (the wire-surface declaration paragraph) and adds Fixes #14937 to the body. The director PASS at 5527222016 stands for a fix commit confined to the ledger row, the changeset and the body; once check:error-code-provenance is green the seat clears the carriers and lands.

Generated by Claude Code

Correction to the line above: the ruling and dispatch order on #14937 is comment 5535600720 (the id I cited was mistyped). Nothing else changes.

Generated by Claude Code

Carriers cleared, landing — director seat, summon 13, session_01WXyGTWPbbreqXow7Z2pZCk, 2026-09-04 ~06:02Z, on the maintainer's direct dispatch 「这两个你帮我派发更新到合并。」 (claim 5535945236 on #14754).

Head bb7da10c2 read on origin/claude/issue-14754-reconcile-per-grant-catch: origin/main merged (the census page regenerated on the committed merge, never in MERGE state); packages/plugins/plugin-sharing byte-identical to the reviewed head 83f307090 (diff empty), so the director PASS at 5527222016 and the seat's adoption at 5528078734 stand for the reviewed content unchanged. The fix commit is confined to what that PASS anticipated: the PROVENANCE_WAIVERS row in packages/spec/src/api/error-code-ledger.zod.ts (package @objectstack/plugin-sharing, code ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED, registeredUnder: '@objectstack/objectql', reason opening "Matches the code, never emits it" and naming #14936 as the removal point — exactly ruling A at 5535600720 on #14937), the changeset's wire-surface paragraph (required patch 1, citations re-measured on the merged tree: ledger row rest-route-ledger.ts:390, handler rest-server.ts:11108-11109, client type client/src/index.ts:4766/4771; #14969 stays the spec follow-up), Fixes #14937 in the body, and the census re-anchoring (required patch 2).

The dev's report on #14754 quotes the gate both ways: before the row, FAIL — 1 stamp site(s) … not listed under its own owner key; after, exit 0, OK — every registered-code stamp site is listed under its own owner key or carries a recorded waiver (9 waiver(s), all live). Spec suite 469 files / 12542 tests green, plugin-sharing 33 files / 811 tests green (the +32 over round 1 arrived with the earlier merge of main, not this round), repo-wide eslint clean, 109 derived gates green with 3 NOT MEASURED on prerequisites.

CI on bb7da10c2: Lint & Repo Gates success (05:36Z — the provenance gate is green in CI), every Type Check job, Build Core, Test Core, Temporal Conformance, Check Changeset, Governed Surface Queue Guard all success.

needs:contract-review cleared on this PR and on #14754 in this stroke (read-modify-write, read back); marked ready; squash auto-merge armed next; followed to MERGED. #14754 and #14937 close with the merge; #14936 stays queued as the durable fix, at which point the waiver row leaves with the stamp site.

Generated by Claude Code